fix(security): reject unsafe frame-ancestor configuration on current main - #430

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Exact-current-head review request for 032b75d2a1bd2c9488d3aa8198a46b0381641ef7 on protected main 961f44a19c4ced87f8062b3ab9842a9a08d05235. CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; the two-file CSP frame-ancestors injection boundary is mergeable with zero unresolved threads. Verify fail-closed semicolon/ISO-control rejection while preserving the 'self' default and valid source lists. Review read-only; do not mutate the branch or bypass independent approval.

@opencode-agent review
@cwl-noema-review review
@strix review
@coderabbitai review

#!/bin/bash
set -euo pipefail

base='961f44a19c4ced87f8062b3ab9842a9a08d05235'
head='032b75d2a1bd2c9488d3aa8198a46b0381641ef7'

printf '%s\n' '== Commit verification =='
git rev-parse HEAD
git show -s --format='head=%H%nparents=%P%nsubject=%s' "$head"
printf 'merge-base=%s\n' "$(git merge-base "$base" "$head")"

printf '%s\n' '== Changed files =='
git diff --name-status "$base" "$head"

printf '%s\n' '== AST outlines for changed Java files =='
while IFS= read -r file; do
  case "$file" in
    *.java)
      printf '\n-- %s --\n' "$file"
      ast-grep outline "$file" --items all --view expanded || true
      ;;
  esac
done < <(git diff --name-only "$base" "$head")

printf '%s\n' '== Complete review patch =='
git diff --no-ext-diff --unified=80 "$base" "$head"

#!/bin/bash
set -euo pipefail

base='961f44a19c4ced87f8062b3ab9842a9a08d05235'
head='032b75d2a1bd2c9488d3aa8198a46b0381641ef7'
changed_list="$(mktemp)"

cleanup() {
  rm -f "$changed_list"
}
trap cleanup EXIT

git diff --name-only "$base" "$head" > "$changed_list"

printf '%s\n' '== AST outlines for changed Java files =='
while IFS= read -r file; do
  case "$file" in
    *.java)
      printf '\n-- %s --\n' "$file"
      ast-grep outline "$file" --items all --view expanded || true
      ;;
  esac
done < "$changed_list"

printf '%s\n' '== Complete review patch =='
git diff --no-ext-diff --unified=100 "$base" "$head"

printf '%s\n' '== Validation-related references in changed files =='
rg -n -C 8 --glob '*.java' \
  '(frame-ancestors|ISO|control|semicolon|;|self|SecurityHeaders)' \
  src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.java \
  src/test/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilterTest.java

@seonghobae Review complete. I found no valid findings.

The checked-out head is 032b75d2a1bd2c9488d3aa8198a46b0381641ef7. Its merge-base is the specified protected-main commit 961f44a19c4ced87f8062b3ab9842a9a08d05235.

The diff changes only the requested filter and focused test. normalizeFrameAncestors rejects semicolons and all ISO control characters before it builds Content-Security-Policy. The behavior preserves the 'self' default and valid custom source lists. The regression test covers both CSP directive injection and newline header injection.

I did not modify the branch or issue an approval. Independent approval requirements remain in effect.

@opencode-agent review
@coderabbitai review
@cwl-noema-review review
@strix review

Review exact head 032b75d2a1bd2c9488d3aa8198a46b0381641ef7 only. Exact-head CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline findings are zero. Validate fail-closed semicolon and ISO-control rejection while preserving the normal 'self' default and valid custom source lists.

Review read-only; preserve independent reviewer identity. Do not mutate the branch, synthesize author approval, weaken protection, or merge.

#!/bin/bash
set -euo pipefail

base='961f44a19c4ced87f8062b3ab9842a9a08d05235'
head='032b75d2a1bd2c9488d3aa8198a46b0381641ef7'

printf '%s\n' '== Commit range =='
printf 'checked_out=%s\n' "$(git rev-parse HEAD)"
printf 'head=%s\n' "$(git rev-parse "$head")"
printf 'merge_base=%s\n' "$(git merge-base "$base" "$head")"
git show -s --format='subject=%s%nparents=%P' "$head"

printf '%s\n' '== Changed files =='
git diff --name-status "$base" "$head"

printf '%s\n' '== Full patch =='
git diff --no-ext-diff --unified=100 "$base" "$head"

printf '%s\n' '== Production and test structure =='
for file in \
  src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.java \
  src/test/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilterTest.java
do
  if [ -f "$file" ]; then
    printf '\n-- %s --\n' "$file"
    ast-grep outline "$file" --items all --view expanded || true
  else
    printf 'Missing expected file: %s\n' "$file"
  fi
done

printf '%s\n' '== Validation and test references =='
rg -n -C 12 --glob '*.java' \
  '(normalizeFrameAncestors|frame-ancestors|ISO.*CONTROL|isISOControl|semicolon|Content-Security-Policy|defaultFrameAncestors)' \
  src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.java \
  src/test/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilterTest.java

printf '%s\n' '== Fuzz-target discovery for relevant security paths =='
rg -n -i -C 3 \
  '(jazzer|clusterfuzz|fuzz.*(header|csp|viewer|security)|ViewerSecurityHeadersWebFilter)' \
  . \
  -g '!**/.git/**' -g '!**/node_modules/**' -g '!**/build/**' -g '!**/dist/**' \
  || true

@seonghobae I reviewed exact head 032b75d2a1bd2c9488d3aa8198a46b0381641ef7 against 961f44a19c4ced87f8062b3ab9842a9a08d05235.

I found one blocking finding.

[P1] Add a discoverable fuzz target for normalizeFrameAncestors

src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.java adds a security-sensitive configuration-to-header parsing boundary. The focused unit test covers one semicolon and one newline payload. The repository’s Jazzer targets do not exercise ViewerSecurityHeadersWebFilter or normalizeFrameAncestors.

AGENTS.md requires Jazzer or ClusterFuzzLite-compatible coverage for security-sensitive parsing/header paths. Add a discoverable fuzz target that supplies arbitrary frame-ancestors strings and verifies that unsafe input fails closed while accepted input cannot add a CSP directive or HTTP header. The existing terminal-success fuzz job cannot provide coverage for this new path without that target.

The implementation otherwise rejects semicolons and ISO control characters before CSP construction. It also preserves the 'self' default and valid custom source lists. I did not modify the branch, approve the PR, weaken protection, or merge.

Learnt from: CR
Repo: ContextualWisdomLab/clearfolio PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-15T01:35:31.643Z
Learning: Applies to **/* : Fuzzing coverage for security-sensitive parsing/header paths must remain
  discoverable through Jazzer or ClusterFuzzLite-compatible targets.

_{You are interacting with an AI system.}

Exact-current-head independent review request for 032b75d2a1bd2c9488d3aa8198a46b0381641ef7.

The two-file CSP configuration boundary is mergeable; CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline findings are zero. Semicolon directive delimiters and every ISO control character fail before response-header construction, while the default and valid custom source-list behavior remains unchanged.

@opencode-agent review
@coderabbitai review
@cwl-noema-review review
@strix review

Review this exact head read-only. Do not mutate the branch, synthesize author approval, weaken protection, or merge. Any base update must regenerate all exact-head evidence.

Rate Limit Exceeded

@seonghobae have exceeded the limit for the number of chat messages per hour. Please wait 26 minutes and 6 seconds before sending another message.